Skip to content

Add multipart requestBody support, update docs #1189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 28, 2023
Merged

Conversation

drwpow
Copy link
Contributor

@drwpow drwpow commented Jun 28, 2023

Changes

Fixes #1123 and #1157 by deprioritizing JSON-like media types (application/json, etc.) and accepting any content type (such as multipart/form-data, application/x-www-form-urlencoded, etc.

Also adds a bodySerializer option to use things like FormData if desired.

How to Review

Tests should pass.

Checklist

  • Unit tests updated
  • README updated

@changeset-bot
Copy link

changeset-bot bot commented Jun 28, 2023

🦋 Changeset detected

Latest commit: 457cd95

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openapi-fetch Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@drwpow drwpow force-pushed the handle-multipart branch from 17ffee4 to 91fc376 Compare June 28, 2023 06:50
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jun 28, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 457cd95
Status: ✅  Deploy successful!
Preview URL: https://38c6eee7.openapi-ts.pages.dev
Branch Preview URL: https://handle-multipart.openapi-ts.pages.dev

View logs

@@ -38,38 +40,48 @@ export type PathsWith<Paths extends Record<string, PathItemObject>, PathnameMeth
}[keyof Paths];
/** Find first match of multiple keys */
export type FilterKeys<Obj, Matchers> = { [K in keyof Obj]: K extends Matchers ? Obj[K] : never }[keyof Obj];
/** handle "application/json", "application/vnd.api+json", "appliacation/json;charset=utf-8" and more */
export type JSONLike = `${string}json${string}`;
export type MediaType = `${string}/${string}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deprioritize application/json and just accept… anything

Of course, this may cause issues where a text/plain comes before application/json. But I’d personally rather have someone put forward that scenario where that happens in the real world.

data = await response.clone().text();
}
const cloned = response.clone();
data = typeof cloned[parseAs] === "function" ? await cloned[parseAs]() : await cloned.text();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor change: don’t catch(); just let the error bubble up if there’s a parsing error

body: typeof requestBody === "string" ? requestBody : JSON.stringify(requestBody),
});
};
if (requestBody) requestInit.body = bodySerializer(requestBody as any);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor change: don’t send body: undefined

@@ -35,7 +35,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [16.x, 18.x, 20.x]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropping Node 16 testing cuz it’ll be EOL in 3 months

@drwpow drwpow merged commit 0380e9a into main Jun 28, 2023
@drwpow drwpow deleted the handle-multipart branch June 28, 2023 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support more type of request body
1 participant